Skip to content

Enable pump heartbeat if needed when adding a pump#2058

Merged
ps2 merged 2 commits into
devfrom
pump-heartbeat-fix
Oct 22, 2023
Merged

Enable pump heartbeat if needed when adding a pump#2058
ps2 merged 2 commits into
devfrom
pump-heartbeat-fix

Conversation

@ps2
Copy link
Copy Markdown
Collaborator

@ps2 ps2 commented Sep 1, 2023

May fix #1854

@ps2 ps2 mentioned this pull request Sep 1, 2023
@marionbarker
Copy link
Copy Markdown
Contributor

marionbarker commented Sep 1, 2023

Test Scenario:

iPhone SE (iOS 16.6) add Dexcom Share as CGM and Pump Simulator:

  • does not update share with Loop at dev branch (commit 3367e60):
    • phone unlocked and app open (starts green) turns yellow - don't wait for red
  • update to Loop branch pump-heartbeat-fix (commit 9b9012f):
    • Loop updates and turns green on rebuild (but that's the same as a quit/restart)
    • phone unlocked and app open - turns yellow and new share value is not populated
    • Confirmed the new line is in DeviceDataManager.swift

Added note: this test was valid. Performed with my SE test phone - there were no future glucose values in Health for this test.

@ps2
Copy link
Copy Markdown
Collaborator Author

ps2 commented Sep 8, 2023

This change only affects the situation described in the issue: add CGM, then pump, and no reboot, or even no loop-background to foreground. It only works with a BLE pump. And only with a ble pump with regular BLE events.

There was an issue where we weren't enabling pump BLE heartbeats when the pump was added after the CGM. That's what this fixes.

@ps2
Copy link
Copy Markdown
Collaborator Author

ps2 commented Sep 22, 2023

@marionbarker I'm not sure if your test is testing the same problem you described in the ticket, as the test report doesn't say anything about adding devices in a particular order. If you're having general issues with a lack of heartbeat, that would be a separate issue that we'd need to look into.

@marionbarker
Copy link
Copy Markdown
Contributor

Latest LoopWorkspace (22 Sep 2023), commit ea23351.

  • add CGM = dexcom share, add Pump = simulator - Loop went red
  • Added the one-line change to DeviceDataManager.swift and rebuilt
    • Loop went green with rebuild
    • Turns yellow at 6 min

If you want a different test - happy to provide one - let me know the desired configuration.

@ps2
Copy link
Copy Markdown
Collaborator Author

ps2 commented Sep 24, 2023

This fix, like the situation described in #1854, is a fix for having a non-ble CGM, and then adding a BLE pump that supports a heartbeat. To test it, you will need to make and install a loop version with the fix, and then add the pump.

@ps2
Copy link
Copy Markdown
Collaborator Author

ps2 commented Oct 16, 2023

The test for this would be:

  1. build & install loop with this change.
  2. add non-ble CGM
  3. add pump with actual heartbeat (ble) support (like Dash or RL based pump)
  4. verify Loop continues to Loop in background.

@marionbarker
Copy link
Copy Markdown
Contributor

I will be happy to test when I return home in a few days.

@marionbarker
Copy link
Copy Markdown
Contributor

Summary: Tested in the order specified by Pete.

Results: This fix makes the app behave correctly when non-heartbeat Dexcom Share CGM is added before heartbeat pump.

  • Nightscout as a Remote CGM works fine even when added before the pump with heartbeat
  • Dexcom Share has the problem before the fix, even with app in foreground
  • Add fix and Dexcom Share works as expected

Test Details

First reproduce the problem, then test the fix:

  1. Start with no app on phone, (iPhone 8, iOS 16.6)
  2. Build dev (without the fix), commit a7cc83c
    • Onboard
    • Add Nightscout CGM (my sensor is warming up, cannot test with Dexcom Share at this time)
    • Add DASH rPi simulator (has a heartbeat)
    • Wait for green loop
    • Lock phone
    • Observe test nightscout site while phone is locked, loop continues to update
  3. Continue with same version of dev, but now my sensor is warmed up
    • Delete CGM and Pump
    • Add Dexcom Share as CGM
    • Add DASH rPi simulator (has a heartbeat)
    • Loop goes yellow even with phone unlocked and app in foreground
  4. Modify Loop/Managers/DeviceDataManager.swift with one line change (PR 2058)
    • Delete CGM and Pump
    • Add Dexcom Share as CGM
    • Add DASH rPi simulator (has a heartbeat)
    • Wait for green loop
    • Lock phone
    • Observe test nightscout site while phone is locked, loop continues to update

@ps2 ps2 merged commit 7c60cac into dev Oct 22, 2023
@ps2 ps2 deleted the pump-heartbeat-fix branch October 22, 2023 19:06
Joerg-Schoemer pushed a commit to Joerg-Schoemer/Loop that referenced this pull request Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

ShareClient fails to update

2 participants